Skip to content

Conversation

@timburks
Copy link
Member

This includes updating the minimum supported iOS and OSX versions
to match gRPC and a fix for a compiler warning found by the linter.

The compiler warning was due to careless conversion of grpc event tags
from void * to various integer types that were 64 bit but inappropriate
for 32 bit architectures. The change now directly exposes void * tags
from the shim layer and converts them from UnsafeMutableRawPointers
to Int (and back) in SwiftGRPC/Core.

This includes updating the minimum supported iOS and OSX versions
to match gRPC and a fix for a compiler warning found by the linter.

The compiler warning was due to careless conversion of grpc event tags
from void * to various integer types that were 64 bit but inappropriate
for 32 bit architectures. The change now directly exposes void * tags
from the shim layer and converts them from UnsafeMutableRawPointers
to Int (and back) in SwiftGRPC/Core.
@timburks
Copy link
Member Author

@MrMage @rebello95 PTAL.

I'm hoping that this podspec is ready to go to the CocoaPods Master Repo.

Copy link
Collaborator

@rebello95 rebello95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 awesome! Thank you for doing this.

type = CompletionType.completionType(event.type)
success = event.success
tag = cgrpc_event_tag(event)
tag = Int(bitPattern:cgrpc_event_tag(event))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Looks like elsewhere in the codebase there's normally a space between the : and the argument - i.e., Int(bitPattern: cgrpc_event_tag(event)). Same below in Handler.swift 🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for catching this


Pod::Spec.new do |s|
s.name = 'SwiftGRPC'
s.version = '0.4.1'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we bump this to 0.4.2?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, will do

}

s.summary = 'Swift gRPC code generator plugin and runtime library'
s.homepage = 'http://www.grpc.io'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to update this to https

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

@rebello95
Copy link
Collaborator

FYI I also have a PR from a few weeks ago open against gRPC Core to fix some void warnings that Swift gets - it's been waiting on a review for a while: grpc/grpc#14854

Copy link
Collaborator

@MrMage MrMage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM save for the things pointed out by @rebello95.

FYI, I've also encountered some build warning with the gRPC CocoaPod (both before and since using gRPC v1.11):
screen shot 2018-04-19 at 09 01 54
I think they are related because the script that builds the gRPC Podspec from a template duplicates some file paths in the Podspec's path fields. I don't have the time to investigate fixing that right now, though ;-)

@timburks timburks merged commit ed76ae8 into master Apr 19, 2018
@timburks
Copy link
Member Author

@rebello95 Thanks for creating grpc/grpc#14854!

@timburks
Copy link
Member Author

@MrMage MrMage deleted the podspec-validation branch September 4, 2018 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants